fix(notification): emit layoutChanged after collapse to refresh ListView#1445
fix(notification): emit layoutChanged after collapse to refresh ListView#1445deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideNotifyModel now emits layoutChanged() after collapsing an app’s notifications so that the ListView fully refreshes its delegates and correctly re-renders displaced notification items. Sequence diagram for collapseApp triggering ListView refresh via layoutChangedsequenceDiagram
actor User
participant NotificationCenterView
participant NotifyModel
participant ListView
User->>NotificationCenterView: clickCollapse(appRow)
NotificationCenterView->>NotifyModel: collapseApp(row)
NotifyModel->>NotifyModel: removeDisplaced transitions
NotifyModel->>NotifyModel: update m_appNotifies
NotifyModel-->>NotificationCenterView: collapseApp returns
NotifyModel-->>ListView: layoutChanged
ListView->>ListView: rebuild all delegates
ListView->>User: render displaced notifications correctly
Updated class diagram for NotifyModel emitting layoutChanged after collapseAppclassDiagram
class NotifyModel {
- m_appNotifies
+ collapseApp(row int) void
+ close() void
+ layoutChanged()
}
class NotificationListView {
- model NotifyModel
+ refreshDelegates() void
}
class AppNotify {
+ id int
+ content
}
class OverlapDelegate {
+ position int
+ render() void
}
NotifyModel "1" o-- "*" AppNotify
NotifyModel "*" o-- "*" OverlapDelegate
NotificationListView --> NotifyModel : uses model
NotificationListView ..> OverlapDelegate : displays
NotificationListView ..> AppNotify : displays
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review这段代码主要是在 1. 语法逻辑
2. 代码质量
3. 代码性能
4. 代码安全
改进建议建议删除 除非 修改后的代码建议: void NotifyModel::collapseApp(int row)
{
// ... 前面的逻辑保持不变 ...
if (overlap) {
beginInsertRows(QModelIndex(), row, row);
m_appNotifies.insert(row, overlap);
endInsertRows();
}
// 删除 emit layoutChanged(); 以避免不必要的全量刷新
}特殊情况处理: 总结: |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Emitting
layoutChanged()without a precedinglayoutAboutToBeChanged()breaks the usual Qt model pattern; consider wrapping the affected operations in a properlayoutAboutToBeChanged()/layoutChanged()pair if the layout truly changes, or use a more targeted signal if it’s only about delegate refresh. - Since
collapseAppalready usesbeginInsertRows/endInsertRows, double-check whether a fulllayoutChanged()is necessary or if a narrower update (e.g.,dataChangedorrowsMoved) can achieve the same visual fix with less impact on view performance.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Emitting `layoutChanged()` without a preceding `layoutAboutToBeChanged()` breaks the usual Qt model pattern; consider wrapping the affected operations in a proper `layoutAboutToBeChanged()`/`layoutChanged()` pair if the layout truly changes, or use a more targeted signal if it’s only about delegate refresh.
- Since `collapseApp` already uses `beginInsertRows`/`endInsertRows`, double-check whether a full `layoutChanged()` is necessary or if a narrower update (e.g., `dataChanged` or `rowsMoved`) can achieve the same visual fix with less impact on view performance.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Root cause: When expanded app notification collapses, removeDisplaced transition blocks delegate updates for displaced items. Next app's overlap delegate appears transparent (position exists but content invisible). Solution: Emit layoutChanged() signal after collapseApp completes. This forces ListView to completely refresh all delegates, ensuring displaced items render correctly. 根本原因:展开的应用通知收起时,removeDisplaced transition阻塞被移位项目的 delegate更新。下一个应用的overlap delegate显示透明(位置存在但内容不可见)。 解决方案:collapseApp完成后发出layoutChanged()信号,强制ListView完全刷新 所有delegate,确保被移位项目正确渲染。 Log: 使用layoutChanged()强制刷新ListView修复收起后delegate不显示问题 PMS: BUG-350789 Influence: 收起超出屏幕的应用通知后,下方通知正常显示
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, re2zero The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/forcemerge |
|
This pr force merged! (status: blocked) |
Root cause: When expanded app notification collapses, removeDisplaced transition blocks delegate updates for displaced items. Next app's overlap delegate appears transparent (position exists but content invisible).
Solution: Emit layoutChanged() signal after collapseApp completes. This forces ListView to completely refresh all delegates, ensuring displaced items render correctly.
根本原因:展开的应用通知收起时,removeDisplaced transition阻塞被移位项目的 delegate更新。下一个应用的overlap delegate显示透明(位置存在但内容不可见)。
解决方案:collapseApp完成后发出layoutChanged()信号,强制ListView完全刷新 所有delegate,确保被移位项目正确渲染。
Log: 使用layoutChanged()强制刷新ListView修复收起后delegate不显示问题
PMS: BUG-350789
Influence: 收起超出屏幕的应用通知后,下方通知正常显示
Summary by Sourcery
Bug Fixes: